If we look at the commit subsys: usb: host: add two APIs for connect/disconnect, it looks like the connect() and disconnect() functions are practically the same.

It is possible to declare the previous functions as public with a prototype in .h to avoid duplicating the code.

If we look at the commit subsys: usb: host: add two APIs for connect/disconnect, it looks like the connect() and disconnect() functions are practically the same.

It is possible to declare the previous functions as public with a prototype in .h to avoid duplicating the code.

Hi @josuah
I have created another commit #100072 about this requirement. Please help to review in this thread. So you can ignore subsys: usb: host: add two APIs for connect/disconnect in current thread.

If this function needs to be used outside, by the HUB, then it is possible to remove the static, give it an API name such as usbh_device_connected(), then add it to usbh_device.h, as an example.

@josuah
Thank you for your feedback.
I am thinking about this solution. If we change this function into a public API, there will be some structural issues. dev_connected_handler is tied to the event handler running in the USB bus thread, triggered by low-level events. Also, the speed information comes from the second parameter event. Making it an API would require dropping or modifying this parameter, which means extra changes.
Instead, if adding a new API in the commit “subsys: usb: host: add two APIs for connect/disconnect”, we can reuse the same logic as the caller to reduce code duplication and keep the structure clean.
This is my opinion, hope your any comment for this topic.

Likewise, if this function needs to be used outside, by the HUB code, then it is possible to remove the static, give it an API name such as usbh_device_remove(), then add it to usbh_device.h.

From the architectural point of view, any hub device is the same usb device as anything else. So that means, that after connection, we already have it in the sys_dlist_t udevs.

What gives us the option to keep the pointer to the "parent", which can be the pointer to the another udev in the list in case the device is a downstream device.
For the device, connected to the root port, it is possible to keep this pointer NULL.

From the uhc logic, there should be no difference, when a new device is connected (to directly to the root port or via external hub port) and it might be handled with the same dev_connected_handler().

This flag (parent == NULL or parent == *another udev) might be used to build the device tree.

Maybe, it makes sense to think about the new abstraction object: device node, which represents the combination: parent + port number? Thus, the root node will be [NULL, 0], and any other node will be [*udev, port_num].

PS. As a bonus, if the hardware has two usb controllers, we can distinguish them by port number, when parent is NULL.

@roma-jam
Thank you for this elegant architectural proposal! I agree that the hub pointer naturally represents the device tree structure, and your device_node abstraction of [parent, port] is conceptually clean. However, I'd like to respectfully point out that while hub devices are indeed USB devices from a data structure perspective, they differ fundamentally in their operational role: hub devices actively manage port events through continuous interrupt endpoint monitoring and complex state machines, whereas regular devices are passive. The current architecture already achieves the unification you're suggesting—both root port and hub port connections ultimately call the same usbh_device_connect() function with a properly initialized usb_device that has its hub and hub_port fields set correctly. The difference is in how the connection is detected (hardware interrupt for root ports vs. software polling for hub ports), not in how the device tree is represented. Regarding your PS about multiple controllers: the current implementation already supports this through separate usbh_context instances per controller, with events carrying the controller information via the event.dev field, so distinguishing controllers by port number when parent == NULL wouldn't be necessary

The current architecture already achieves the unification you're suggesting—both root port and hub port connections ultimately call the same usbh_device_connect() function with a properly initialized usb_device that has its hub and hub_port fields set correctly

The one thing I still wonder about - what can be used during enqueuing to tell the driver, that this particular device is connected via hub?

Some drivers required this knowledge to configure the channel correctly.

To do this, and to handle this correctly, we need to be able to get the following params:

• parent port speed [speed of the hub]
• device speed [speed of the port, when it is enabled]

This is important to support combinations as High speed hubs + Low speed devices, connected to Hubs port.

Is there any way to get these params?

Good question! The current implementation already provides access to both parameters you mentioned through the struct usb_device pointer:

Parent port speed (Hub's speed): Available via udev->hub->speed
Device speed: Available via udev->speed
The struct usb_device contains the complete topology information including the parent hub pointer. For example, in the MCUX driver, we use USB_HostHelperGetPeripheralInformation() to query these parameters:

// Get device speed
USB_HostHelperGetPeripheralInformation(udev, kUSB_HostGetDeviceSpeed, &device_speed);

// Get HS hub address (if device is LS/FS connected to HS hub)
USB_HostHelperGetPeripheralInformation(udev, kUSB_HostGetDeviceHSHubNumber, &hs_hub_addr);

// Get HS hub port
USB_HostHelperGetPeripheralInformation(udev, kUSB_HostGetDeviceHSHubPort, &hs_hub_port);

These helper functions traverse the udev->hub chain to find the high-speed hub and extract the necessary TT-related information for handling HS Hub + LS/FS device combinations. The driver can access all the topology information it needs through the device pointer to properly configure channels for different speed combinations.

Maybe, it makes sense to introduce the whole hub-related part of the struct? Or to introduce them as "getters", because they are part of the Hub Descriptor that we need to request in the potential class driver?

Because, there are several other important hub characteristics, that we might be able to use in the driver.

Such as:

• number of ports (relevant, when we handle the port(s) connection)
• time from power on to power good (relevant, when hub can power on each port or all ports altogether)
• characteristics (tt includes here, as well as port logical switching and led support)
• current consumption

Thank you for the comment here,

Actually previously the total uint16_t hub_characteristics; is added here, but @jfischer-no suggests keep think time here.

Just out of curiosity, what is the purpose of having level in explicit form?

The level field is essentially a topology depth indicator that helps the USB stack:
Enforce USB specification limits on hub chaining
Provide necessary information to the hardware controller driver
Maintain and validate the device tree structure

This file seems like a candidate to be usb_ch11.h, not usb/class/usb_hub.h?

Yes, agree, updated.

Some hubs might have several interfaces with different TT. Especially on High speed configuration.

Chapter 11 provides the diagram of downstream port in Figure 11-10. Downstream Facing Hub Port State Machine.

This is a part of it:

Maybe it is better to follow names and port states from the specification to provide easier maintenance in the future?

There are 14 k_work_submit() throughout the code. Maybe, it might be done simpler, because usually the hub require port handling only after port status changed and ports have quite fixate determine states (according the the USB 2.0 Figure 11-10). So it allows to handle them one by one till they all are in any final state (powered off, disconnected [no device], connected [port has a device and device completed the reset] or disabled [overcurrent or unable to reset the device in port])

Maybe, with the following approach it might be simplified to two (or at elast three) k_work_sumbits:

• k_work_submit(&hub_mgr_data->hub_work...): New hub appeared -> we need to get descriptor, configure the internal object and prepare ports for handling (allocate their objects and put in the list of pending ports), enable the interrupt EP IN to be able to catch the port or hub changes. (We need to be careful, as the hub already might have a device attached, so we can postpone enabling EP IN till we handle the port with a device - it might be another Hub to enable)

• k_work_submit(&hub_mgr_data->port_work...): Hubs port/ports have something, that needs to be handled. Usually, starts after the EP IN transfer complete. We can add he port to pending list and then the work might handle the pending port list till it is empty. Resubmit the transfer again when no more port to handle.

This might help to simplify the logic and make it more linear.
Otherwise it is easy to miss the important details or maintain the code IMHO.

There are two more possible port states: Powered Off and Disabled.

We can merge the PORT_STATE_ERROR with PORT_STATE_DISABLED, but this limits the legitimate state of changing the port to disable state by ClearPortFeature(PORT_ENABLE).

This commit needs to be fixed, we cannot just remove these functions. I see you moved them to usbh_hub.[c,h], but IMO the right place would be usbh_ch11.[c,h]. What do you think?
Header include in the shell needs to be update as well. Also see my comment in the next commit.

I suggest splitting this commit into two. The introduction of the chapter 11 control requests above should be moved to the previous commit (where you removed the two existing hub request). These changes would then be atomically and bisectable. After that, the commit that introduces hub class would be more clear.